-
Notifications
You must be signed in to change notification settings - Fork 90
Develop #788
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Develop #788
Conversation
tz = "UTC"){ | ||
|
||
.Deprecated(new = "read_waterdata_samples", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this show up if the call errors? I find it interesting that it prints after the data are returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Google's AI is right (and I think it is), it depends on:
options(warn = 0) (default): Warnings are stored and printed after the top-level function completes.
options(warn = 1): Warnings are printed as they occur.
options(warn = 2): Warnings are treated as errors and will halt execution.
This is from ?options
integer value to set the handling of warning messages by the default warning handler. If warn is negative all warnings are ignored. If warn is zero (the default) warnings are stored until the top–level function returns. If 10 or fewer warnings were signalled they will be printed otherwise a message saying how many were signalled. An object called last.warning is created and can be printed through the function [warnings](http://127.0.0.1:40507/help/library/base/help/warnings). If warn is one, warnings are printed as they occur. If warn is two (or larger, coercible to integer), all warnings are turned into errors. While sometimes useful for debugging, turning warnings into errors may trigger bugs and resource leaks that would not have been triggered otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, had to convince myself. Learn more about R every day.
test <- function(x){
.Deprecated(new = "test1")
return(x-1)
}
test(1)
[1] 0
Warning message:
In test(1) : 'test' is deprecated.
Use 'test1' instead.
See help("Deprecated")
test("stuff")
Error in x - 1 : non-numeric argument to binary operator
In addition: Warning message:
In test("stuff") : 'test' is deprecated.
Use 'test1' instead.
See help("Deprecated")
test(y="stuff")
Error in test(y = "stuff") : unused argument (y = "stuff")
Co-authored-by: Elise Hinman <121896266+ehinman@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I tested out read_waterdata_daily
, read_waterdata_ts_meta
, read_waterdata_monitoring_location
. I also tried out read_USGS_samples
to see how it would behave and how it would warn me that I should start using read_waterdata_samples
. I tried to get it to error to see if the warning would still be printed, but I was unsuccessful (which I guess is good?). I just tried this really obvious error and it doesn't print out the warning:
read_USGS_samples(derp = "dog")
Error in read_USGS_samples(derp = "dog") : unused argument (derp = "dog")
I looked at the help for read_USGS_samples
and it automatically took me to the help for read_waterdata_samples
, which might be a little bit confusing if I haven't yet run read_USGS_samples
. I don't know if it would make sense to add somewhere in the description that it redirects from read_USGS_samples
because you shouldn't use it. Same for summarize_USGS_samples
.
I played around with the setAccess
function and think that the warning message is sufficient. Also verified that I could see the access in the waterservices web service url 😛 .
I also tried building the documentation using pkgdown::build_articles()
, and it eventually threw an error:
Reading vignettes/long_to_wide.Rmd
Error in `map()`:
ℹ In index: 4.
Caused by error in `.f()`:
✖ Failed to render RMarkdown document.
Quitting from lines 66-75 [twoSites] (long_to_wide.Rmd)
Caused by error:
! could not find function "read_waterdata_samples"
Not sure if this is on my end or if there's something missing somewhere...I just used devtools::load_all()
to access the functions.
To get a vignette to build with the latest/greatest package, I've always needed to to the full "Install" vs https://water.code-pages.usgs.gov/dataRetrieval/articles/long_to_wide.html |
Co-authored-by: Elise Hinman <121896266+ehinman@users.noreply.github.com>
No description provided.